Skip to content

RAJA::launch sycl, tiling, and reduction tests #1473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
artv3 merged 56 commits into from
Jun 20, 2023
Merged

RAJA::launch sycl, tiling, and reduction tests #1473

artv3 merged 56 commits into from
Jun 20, 2023

Conversation

@artv3
Copy link
Member

@artv3 artv3 commented Apr 26, 2023

Turning on SYCL, and adding reduction tests.

@artv3 artv3 mentioned this pull request May 12, 2023
Closed
@artv3 artv3 changed the title More launch tests RAJA::launch sycl, tiling, and reduction tests May 17, 2023
@artv3
Copy link
Member Author

artv3 commented Jun 14, 2023

Hi all, could I get a review on this PR? I would like to merge it along with #1472

@artv3
Copy link
Member Author

artv3 commented Jun 15, 2023

The last CI run threw the following failure:

tests/test-forall-AtomicView.hpp:74: Failure
Expected equality of these values:
  (T)2
    Which is: 2
  check_array[i]
    Which is: 0
    ```

@artv3
Copy link
Member Author

artv3 commented Jun 16, 2023

@MrBurmark @rchen20 @rhornung67 , @mdavis36 -- good news all green! Did anyone else want to take a look or are we good to merge?

@rhornung67
Copy link
Member

rhornung67 commented Jun 20, 2023

@artv3 this is good to merge. If you want someone else to approve, please ping them.

@artv3 artv3 merged commit 4b7d3b5 into develop Jun 20, 2023
@artv3 artv3 deleted the artv3/more-launch-test branch June 20, 2023 17:41
rchen20
rchen20 reviewed Jun 20, 2023
View reviewed changes
test/functional/launch/reduce-basic/tests/test-launch-basic-ReduceBitAnd.hpp

RAJA::launch<LAUNCH_POLICY>
(RAJA::LaunchParams(RAJA::Teams(blocks), RAJA::Threads(threads)),
[=] RAJA_HOST_DEVICE(RAJA::LaunchContext ctx) {
Copy link
Member

@rchen20 rchen20 Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: I've been using [&] to capture the local variables by reference, per the original design of RAJA::Teams. Did something change where now the best practice for Launch is to capture by value, or is capture by value just used for these simple tests?

Copy link
Member

@rhornung67 rhornung67 Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe capture-by-value is used on the outer lambda passed to the RAJA::launch method, and lambdas internal to that use capture-by-reference. @artv3 please correct me if I'm wrong.

Copy link
Member Author

@artv3 artv3 Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The outer lambda is capture by value, inner lambdas are capture by reference.

Copy link
Member

@rchen20 rchen20 Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MrBurmark MrBurmark MrBurmark left review comments

@rhornung67 rhornung67 rhornung67 approved these changes

@rchen20 rchen20 rchen20 left review comments

@mdavis36 mdavis36 Awaiting requested review from mdavis36

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@artv3 @rhornung67 @MrBurmark @rchen20